fix(customresourcestate): generate unique HELP message for each family #2635
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
This issue is currently awaiting triage. If kube-state-metrics contributors determine this is a relevant issue, they will accept it by applying the The Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
k8s-ci-robot
added
size/XS
|
/assign @rexagod |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
@rexagod Anything I can do to help here? |
There was a problem hiding this comment.
@mrueg Do you see this as a breaking change? We don't offer any guarantees and such on the metric metadata, as such, the metrics stability framework only applies to a metrics' label-set, but this may still break things for the user (unofficial use-cases with grepping, etc.). Additionally, this makes the user hard opt-into a custom HELP text.
| @@ -90,10 +90,13 @@ func compileFamily(f Generator, resource Resource) (*compiledFamily, error) { | |||
| if errorLogV == 0 { | |||
| errorLogV = resource.ErrorLogV | |||
| } | |||
|
|
|||
| help := fmt.Sprintf("%s for %s/%s/%s", f.Help, resource.GroupVersionKind.Group, resource.GroupVersionKind.Version, resource.GroupVersionKind.Kind) | |||
There was a problem hiding this comment.
Lets stick to the standardized representation here: https://github.com/kubernetes/apimachinery/blob/v0.33.0/pkg/apis/meta/v1/group_version.go#L93.
k8s-ci-robot
removed
the
lgtm
|
New changes are detected. LGTM label has been removed. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: CatherineF-dev, Duologic The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
@rexagod Anything I can help to get this merged? |
Duologic
force-pushed
the
duologic/wildcard_duplicates_removed
branch
from
c556dcd to
2e19a83
Compare
|
rebased. |
The family generator creates a family per GVK, this requires that the configuration has a unique HELP text per custom metric. This is not feasible when using wildcards as the generate will reuse the same HELP text for each GVK it finds.
Duologic
force-pushed
the
duologic/wildcard_duplicates_removed
branch
from
2e19a83 to
ebf0fdb
Compare
|
rebased and fixed unit test |
What this PR does / why we need it:
The family generator creates a family per GVK, this requires that the
configuration has a unique HELP text per custom metric. This is not
feasible when using wildcards as the generate will reuse the same HELP
text for each GVK it finds.
How does this change affect the cardinality of KSM: (increases, decreases or does not change cardinality)
It does not change cardinality but does change the HELP text on metrics generated by CustomResourceState.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #2384, fixes #2453